Skip to content

Conversation

@albertzaharovits
Copy link
Contributor

@albertzaharovits albertzaharovits commented Sep 13, 2024

PoC

Comment on lines +544 to 548
// the difference between 16 and 18 effectively results in the same "encoded" API Key that's sent in HTTP request headers,
// dues to base64 padding
final SecureString apiKey = getBase64SecureRandomString(request.getType() == ApiKey.Type.CROSS_CLUSTER ? 16 : 18);
assert ApiKey.Type.CROSS_CLUSTER != request.getType() || API_KEY_SECRET_LENGTH == apiKey.length()
: "Invalid API key (name=[" + request.getName() + "], type=[" + request.getType() + "], length=[" + apiKey.length() + "])";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @n1v0lg pointed out, we'd need to support the current length for cross cluster API keys.
Maybe other (external, i.e. non-ES) consumers rely on the "api_key" length not changing, but hopefully they use the "encoded" one in the API response, which doesn't change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g.:

{
  "id" : "bZ0Y7JEBuvKKBRFbe4xD",
  "name" : "test",
  "api_key" : "BGEQu0kTIU8x7b63k9Np41dE",
  "encoded" : "YlowWTdKRUJ1dktLQlJGYmU0eEQ6QkdFUXUwa1RJVTh4N2I2M2s5TnA0MWRF"
}

The encoded is still 60 chars, but api_key changes from 22 to 24.

.map(Hasher::name)
.map(name -> name.toLowerCase(Locale.ROOT))
.filter(name -> (name.startsWith("pbkdf2") || name.startsWith("bcrypt")))
.filter(name -> (name.startsWith("pbkdf2") || name.startsWith("bcrypt") || "ssha256".equals(name)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be allowed only for API keys, not for passwords (api keys are random long secrets).

public static final Setting<String> PASSWORD_HASHING_ALGORITHM = XPackSettings.defaultStoredHashAlgorithmSetting(
"xpack.security.authc.api_key.hashing.algorithm",
(s) -> Hasher.PBKDF2.name()
(s) -> Hasher.SSHA256.name()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is currently no PBKDF2 option with cost lower than 1000. If we add a new hasher type/cost we need to worry about BWC (old node not understanding keys generated by new nodes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants